Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add language select, abstract text transformations #584

Merged
merged 101 commits into from
Feb 17, 2024

Conversation

StefanVukovic99
Copy link
Member

Adds a language select to Options > General, and makes text transformations language-specific.

I don't fully understand the offscreen proxy stuff, but i think it was necessary. The LanguageUtil on manifest v2 dynamically loaded files the first time a language was called for. Seems modules can't be loaded dynamically in the "backend" on MV3, so any suggestions to that are welcome, and as always to the naming/types.

Part 6 of #422. After this, most Yezichak users should be able to move to Yomitan.

@StefanVukovic99 StefanVukovic99 requested a review from a team as a code owner January 28, 2024 16:21
Copy link

github-actions bot commented Jan 28, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@toasted-nutbread
Copy link

What is the delta you're seeing? The comment above is only showing N/A, not sure where to see accurate numbers.

themoeway-bot
themoeway-bot previously approved these changes Feb 12, 2024
@StefanVukovic99
Copy link
Member Author

What is the delta you're seeing? The comment above is only showing N/A, not sure where to see accurate numbers.

image

Yeah, the changes to the test make it lose track, but it's gone from about 9 ms per lookup to about 24.

@toasted-nutbread
Copy link

RIP having to deconflict for the umpteenth time - #636 also added a setting upgrade.

djahandarie
djahandarie previously approved these changes Feb 15, 2024
Copy link
Collaborator

@djahandarie djahandarie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's think about how to improve performance in further PRs. Though I think we need to make sure we have the right set of benchmarks that really indicate end-to-end performance (esp when there are a lot of dictionaries loaded) rather than just microoptimizing the existing benchmarks we have.

I will merge after @toasted-nutbread's final review

@djahandarie djahandarie added this pull request to the merge queue Feb 17, 2024
github-merge-queue bot pushed a commit that referenced this pull request Feb 17, 2024
* Copy functions from JapaneseUtil

* Remove JapaneseUtil

* Update usages of JapaneseUtil functions

* part1

* frotend done?

* fix tests

* offscreen and type complications

* add tests

* start fixing tests

* keep fixing tests

* fix tests

* Copy functions from JapaneseUtil

* Remove JapaneseUtil

* Update usages of JapaneseUtil functions

* delete pt

* renames

* add tests

* kebab-case filenames

* lint

* minor fixes

* merge

* fixes

* fix part of comments

* fix more comments

* delete unused types

* comment

* comment

* do backend

* other files

* move fetch utils to own file

* remove extra line

* add extra line

* remove unnecessary export

* simplify folder structure

* remove redundant async

* fix param type in api

* fix language index

* undo changes to cssStyleApplier

* undo changes to utilities.js

* undo changes to utilities.js

* simplify language util

* lint

* undo phantom changes to anki integration

* require textTransformations options

* explicit locale in localeCompare

* punctuate notes

* prefer early exit

* rename LanguageOptionsObjectMap

* rename to textPreprocessor

* tuple with names instead of boolean array

* safe data setting

* optional chaining

* simplify LanguageOptions

* encapsulate languages

* delete language util

* nullable language in text preprocessors controller

* rename transform to process

* remove settings

* make translation advanced again

* remove unused getTextTransformations api call

* comments

* change language types

* RIP flags

* comments

* fix tests

* lint

* Text preprocessor type changes (FooSoft#10)

* Add types

* Update types

* Simplify type check

* Refactor typing and structuring of language definitions

* lint

* update translator benchmark

* undo markdown changes

* undo markdown changes

* undo markdown changes

* more merge

* simplify language controller

---------

Co-authored-by: toasted-nutbread <toasted-nutbread@users.noreply.github.com>
Co-authored-by: Darius Jahandarie <djahandarie@gmail.com>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 17, 2024
@djahandarie djahandarie added this pull request to the merge queue Feb 17, 2024
Merged via the queue into themoeway:master with commit 4aaa9f1 Feb 17, 2024
7 of 8 checks passed
@djahandarie djahandarie added the kind/enhancement The issue or PR is a new feature or request label Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement The issue or PR is a new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants